Fix/ans spec compliance by busehalis-sap · Pull Request #34 · cap-java/cds-feature-notifications

busehalis-sap · 2026-06-23T14:01:43Z

Summary

This PR aligns the notification provisioning logic with the ANS API specification.

- NotificationType payload now uses Translations instead of embedded Templates: The ANS API requires Translations for user-facing display names and group titles. @notification.template.groupedTitle is now a required annotation, startup fails with a clear error if it is missing.
- Delete+create strategy for re-provisioning: ANS rejects payloads that mix Translations and Templates on an existing type (HTTP 400). Replacing UPDATE with delete+create avoids this rejection for NotificationType. The same strategy is applied to NotificationTemplate for consistency.
- Locale filtering per event: Translations are now built only for locales that have app-specific notification texts, filtering out framework-only locales (e.g. 37 languages from @sap/cds/common) that would produce duplicate English content.
- Explicit PRIVATE visibility on templates: When @notification.customizable is absent, PRIVATE is now set explicitly instead of relying on ANS default behavior.
- Null-safe production mode check: cds.environment.production.enabled now correctly defaults to local mode when the property is not configured (previously unboxing a null Boolean could cause a NullPointerException).
- System.out.println replaced with logger.info in LocalNotificationTypeAutoProvisionerHandler: local mode notification type logs now go through the standard logging framework instead of stdout.
- NotificationBuilderTest renamed to NotificationAssemblerTest and moved to assemblers package: the test class was testing NotificationAssembler, not a builder, so the name and location are now aligned with the class under test.

…set explicit PRIVATE visibility when @notification.customizable is absent

… and move to assemblers package

…ationTypeAutoProvisionerHandler

…d validation and ANS spec compliance

…Type re-provisioning to avoid ANS 400 rejection when existing type has Templates but new payload uses Translations, and align NotificationTemplate re-provisioning to use the same strategy for consistency

…ntries

…cationType Translations instead of @notification.template.title, and throw on missing required annotation

…tionTemplate re-provisioning

…NotificationTemplate re-provisioning" This reverts commit 8590d10.

…ranslations instead of embedded Templates

…s not part of the ANS API spec

…oning

…ingTest as DisplayName is no longer set

lisajulia

Thanks for the PR :) please have a look at my findings!

lisajulia · 2026-06-24T07:35:57Z

-    // Visibility - from @notification.customizable annotation (ANS defaults to PRIVATE)
-    // @notification.customizable: true → PUBLIC, absent or false → PRIVATE (default)
-    String visibility = extractVisibility(event);
-    if (visibility != null) {


Is this check not needed anymore?

The default value from ANS is PRIVATE when no visibility is set. I updated the extractVisibility method to handle this explicitly: if the @notification.customizable annotation is absent, it now returns "PRIVATE" directly instead of null. So the method always returns either "PUBLIC" or "PRIVATE", making the null check here unnecessary.

lisajulia · 2026-06-25T04:40:51Z

+          e.getMessage());
    }
+
+    createNotificationType(notificationType);


So updating a nofication type works like this: updateNotificationType calls DELETE and then createNotficationtype.
What happens if the DELETE fails, then the code will print a warning, failed to delete and try that again - I think we can have an infinite loop here... please add a maxCall or sth. else to prevent this.
If I'm wrong, add at least a test for this, i.e. failing on DELETE.

Thanks for the hint! Just to clarify the history: this was originally using PATCH, but i switched to delete+create in this PR because existing types in ANS had embedded Templates, and the ANS spec does not allow mixing Translations with Templates on an existing type, so PATCH was being rejected with 400. However, delete+create introduces the infinite loop risk you mentioned. Since all types will have Translations after the first successful provisioning, the Templates mixing issue only occurs during initial migration from old types. For that, old types simply need to be deleted manually from ANS once. I have now switched back to PATCH, which is safer and cleaner: no delete risk, no recursive call chain.

lisajulia · 2026-06-25T04:42:39Z

+          e.getMessage());
    }
+
+    createTemplate(template);


Can this become an infinite loop? Same problem as in NotificationTypeAutoProvisionerHandler.

You're right that there's a loop risk here. I explored alternatives but had to stick with delete+create:

Unlike NotificationType, ANS exposes only a PUT endpoint for NotificationTemplate updates, no PATCH. CDS Update.entity().data() is translated to PATCH on remote OData v2 services, so ANS rejects it with 501 Not Implemented. I also tried Upsert.into().entry() but it's not supported on remote OData services either (No ON handler completed the processing). There's no CDS statement that translates to PUT for remote OData services.

So delete+create remains the only viable option here. To prevent the infinite loop, I broke the recursive chain in createTemplate: when a 409 is encountered, instead of calling updateTemplate (which would DELETE+CREATE again and could 409 forever if DELETE keeps failing), i now log a warning and skip. The template stays as-is in ANS for that startup and will be retried on the next one.

lisajulia · 2026-06-25T04:53:46Z

+
+    Set<Locale> filtered = new LinkedHashSet<>();
+    for (Locale locale : allLocales) {
+      // Always keep root ("und" = fallback for unknown locales) and English


What is word "und" in this case?

'und' is the BCP-47 tag which stands for "undefined/undetermined language", produced by Locale.ROOT.toLanguageTag() in Java. CDS represents the default i18n.properties file (without a language suffix) as Locale.ROOT, which results in 'und'.

Since 'und' is not a valid ANS language code, I have now mapped Locale.ROOT to Locale.ENGLISH ('en') before building translations. Thanks for the hint!

lisajulia · 2026-06-25T05:04:01Z

+    return translations;
+  }
+
+  private static String truncate(String value, int maxLength) {


Possibly expose a warning here if some things are truncated?

Thanks for the suggestion! I tested this with a value exceeding 256 characters. ANS returns a 400 Bad Request which causes a startup failure with a clear error message including the ANS validation details. I decided to keep the explicit failure instead of silent truncation, because truncating would send a shorter value than what the developer defined without them being aware of it. So I removed the truncate method and also improved the 400 error message to mention that field values may exceed ANS length limits, so the developer gets a clearer hint on what to fix.

lisajulia · 2026-06-25T05:04:49Z

-    notificationType.setNotificationTypeId(notificationTypeId);
    logger.debug(
-        "Updating NotificationType '{}' (id={})",
+        "Updating NotificationType '{}' (id={}) via delete+create",


Did you mean to change the log message from 'via delete+create' to 'via delete+recreate'?

…format, batch index, and email body rendering

…ForEvent as 'und' is not a valid ANS language code

…back to PATCH to eliminate infinite loop risk

…Ds stable instead of delete+create

… skipping update on 409

busehalis-sap added 10 commits June 23, 2026 15:23

fix: use null-safe check for production mode to default to local mode

6943929

feat: filter locales per event to exclude framework-only locales and …

2090b0d

…set explicit PRIVATE visibility when @notification.customizable is absent

refactor: rename NotificationBuilderTest to NotificationAssemblerTest…

842a0b6

… and move to assemblers package

refactor: replace System.out.println with logger.info in LocalNotific…

938ffb9

…ationTypeAutoProvisionerHandler

feat: add Translations to NotificationType payload with required fiel…

0119525

…d validation and ANS spec compliance

refactor: replace UPDATE with delete+create strategy for Notification…

de22798

…Type re-provisioning to avoid ANS 400 rejection when existing type has Templates but new payload uses Translations, and align NotificationTemplate re-provisioning to use the same strategy for consistency

docs: update CHANGELOG with ANS spec compliance changes and missing e…

7b39260

…ntries

chore: apply spotless formatting

036b2c2

test: add unit tests for NotificationTypeAssembler

f96ffca

chore: apply spotless formatting to NotificationTypeAssemblerTest

3218fc5

busehalis-sap requested a review from lisajulia June 23, 2026 14:22

busehalis-sap added 5 commits June 24, 2026 10:20

fix: use @notification.template.publicTitle for DisplayName in Notifi…

29d3208

…cationType Translations instead of @notification.template.title, and throw on missing required annotation

chore: apply spotless formatting

50f751f

refactor: replace delete+create with PUT update strategy for Notifica…

8590d10

…tionTemplate re-provisioning

Revert "refactor: replace delete+create with PUT update strategy for …

1a0b71c

…NotificationTemplate re-provisioning" This reverts commit 8590d10.

docs: update CHANGELOG to remove implementation details

a98815b

busehalis-sap force-pushed the fix/ans-spec-compliance branch from 1a3c5b7 to a98815b Compare June 24, 2026 11:02

busehalis-sap added 5 commits June 24, 2026 14:22

refactor: update LocalNotificationTypeAutoProvisionerHandler to log T…

b991a77

…ranslations instead of embedded Templates

fix: remove DisplayName from NotificationTemplate translation as it i…

f6e714d

…s not part of the ANS API spec

refactor: improve local mode logging for NotificationTemplate provisi…

669a921

…oning

chore: apply spotless formatting

e32fc1e

test: remove DisplayName assertion from NotificationTemplateProvision…

5b76e02

…ingTest as DisplayName is no longer set

lisajulia requested changes Jun 25, 2026

View reviewed changes

busehalis-sap added 7 commits June 25, 2026 09:55

refactor: improve local mode logging for LocalHandler with box-style …

48f09f3

…format, batch index, and email body rendering

chore: apply spotless formatting

9688fbd

fix: map Locale.ROOT ('und'=undefined) to 'en' in getAvailableLocales…

7a6ebe3

…ForEvent as 'und' is not a valid ANS language code

fix: improve 400 error message to mention ANS field length limits

26d45f3

refactor: switch NotificationType re-provisioning from delete+create …

2f0d052

…back to PATCH to eliminate infinite loop risk

test: update NotificationTypeProvisioningTest to verify PATCH keeps I…

939ea29

…Ds stable instead of delete+create

fix: prevent infinite loop in NotificationTemplate re-provisioning by…

3fd9463

… skipping update on 409

busehalis-sap requested a review from lisajulia June 26, 2026 15:39

Uh oh!

Conversation

busehalis-sap commented Jun 23, 2026

Summary

Uh oh!

lisajulia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

busehalis-sap Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

busehalis-sap Jun 26, 2026 •

edited

Loading